Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GeoMechanicsApplication] A 3D elastic constitutive law is lacking in GeoMechanicsApplication #12816

Merged
merged 14 commits into from
Nov 8, 2024

Conversation

markelov208
Copy link
Contributor

@markelov208 markelov208 commented Nov 1, 2024

📝 Description
A brief description of the PR.

  • renamed GeoLinearElasticPlaneStress2DLaw to GeoIncrementalLinearElasticLaw
  • added Elastic3D() law like PlaneStrain()
  • registered GeoLinearElastic3DLaw
  • added unit tests for GeoLinearElastic3DLaw
  • replaced LinearElastic3DLaw with GeoLinearElastic3DLaw for integration tests

@markelov208 markelov208 self-assigned this Nov 1, 2024
@AlejandroCornejo
Copy link
Member

AlejandroCornejo commented Nov 2, 2024

it is none of my bussiness ;) but would it be possible to generalize the existing CLs from the SMApp to satisfy your needs?

Copy link
Contributor

@WPK4FEM WPK4FEM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Gennady,
Thank you for this effective continuation of the incremental elastic material setup. The important remark in my small comments is probably the one about the incremental nature.
Regards, Wijtze Pieter

}

bool& GeoLinearElasticPlaneStrain2DLaw::GetValue(const Variable<bool>& rThisVariable, bool& rValue)
bool& GeoIncrementalLinearElasticLaw::GetValue(const Variable<bool>& rThisVariable, bool& rValue)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should find out what Stenberg shear stabilization is and whether it is applicable for elasticity. We are moving this around, but we are not really using it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked at Stenberg stabilization. Only seems applicable for thick shell formulation used in Structural Mechanics Applications. I consider it to be part of another PR to limit its use to meaningful use in GeoMechanics.

{
// This Constitutive Law has been checked with Stenberg Stabilization
if (rThisVariable == STENBERG_SHEAR_STABILIZATION_SUITABLE) rValue = true;
return rValue;
}

void GeoLinearElasticPlaneStrain2DLaw::GetLawFeatures(Features& rFeatures)
void GeoIncrementalLinearElasticLaw::GetLawFeatures(Features& rFeatures)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation reads like SetFeatureOptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed ;) It is used across Kratos.

@@ -53,7 +53,7 @@ class KRATOS_API(GEO_MECHANICS_APPLICATION) LinearElastic2DBeamLaw : public GeoL
using CLBaseType = ConstitutiveLaw;

/// The base class ElasticIsotropicK03DLaw type definition
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rotting comment, was already wrong before the current change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

corrected

@@ -53,7 +53,7 @@ class KRATOS_API(GEO_MECHANICS_APPLICATION) LinearElastic2DInterfaceLaw : public
using CLBaseType = ConstitutiveLaw;

/// The base class ElasticIsotropicK03DLaw type definition
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same wrong comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

corrected

KRATOS_EXPECT_VECTOR_RELATIVE_NEAR(expected_stress, stress, 1e-3);
}

KRATOS_TEST_CASE_IN_SUITE(GeoLinearElastic3DLawReturnsExpectedStress_WhenOnlyDiagonalEntriesAreConsidered,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, really good to test this as it is a necessity for doing the K0 things in 3D.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All credits to Richard. I've just applied his tests.

@@ -0,0 +1,175 @@
// KRATOS___
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The incremental nature of the law is not tested yet. That would require two steps and change of the Young's modulus in between, to distinguish between an incremental and a total formulation.

Copy link
Contributor

@rfaasse rfaasse Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is tested for the GeoIncrementalLinearElasticLaw with a 2D ConstitutiveDimension though, since the incremental nature is not really dimension-specific, that could be enough, let's have a discussion on this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should have read more carefully, Richard has shown me that the testing is there also for 3D.

Copy link
Contributor

@rfaasse rfaasse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very neat and clear PR, thank you for your effort and delivering a PR this quickly!

The comments I have are mainly about naming and docstrings, so easy to fix (but still hard to find the correct names). Other than that I think it'd be a good idea to discuss the testing of the incremental nature (i.e. are the unit tests for the law with 2D behavior enough or do we need to do a similar thing for 3D).

@@ -351,6 +351,7 @@ void KratosGeoMechanicsApplication::Register()
KRATOS_REGISTER_CONSTITUTIVE_LAW("LinearElasticPlaneStrainK02DLaw", mLinearPlaneStrainK0Law)
KRATOS_REGISTER_CONSTITUTIVE_LAW("LinearElasticK03DLaw", mElasticIsotropicK03DLaw)
KRATOS_REGISTER_CONSTITUTIVE_LAW("GeoLinearElasticPlaneStrain2DLaw", mLinearElasticPlaneStrain2DLaw)
KRATOS_REGISTER_CONSTITUTIVE_LAW("GeoLinearElastic3DLaw", mLinearElastic3DLaw)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should have the 'incremental' specifier in the registered name (maybe GeoIncrementalLinearElastic3DLaw?), but it's good to also have @WPK4FEM's opinion on this)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree to have "Incremental" in the registered name.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I share Richards point of view.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added Incremental

namespace Kratos
{

class KRATOS_API(GEO_MECHANICS_APPLICATION) Elastic3D : public ConstitutiveLawDimension
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm doubting a bit about the name of the class, the plane strain counterpart is just called PlaneStrain, but I'm not sure if ThreeDimensional would be descriptive enough.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favor of renaming this class to ThreeDimensional. Needless to say that we then also need to rename this header file as well as the corresponding implementation file.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do @rfaasse and @WPK4FEM think about ElasticIsotropic3D and to change ConstitutiveLawDimension to ConstitutiveLawType? For example, there is GeoLinearElasticPlaneStress2DLaw which fits this structure perfectly using PlaneStress like PlaneStrain. Another candidate is ElasticIsotropicK03DLaw. Thank you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to ThreeDimensional. However, looking to a discussion on my suggestion. ;)

Comment on lines 24 to 27
* @class GeoIncrementalLinearElasticLaw
* @ingroup GeoMechanicsApplication
* @brief This class defines a small deformation linear elastic constitutive model for plane strain cases
* @brief This class defines a small deformation linear elastic constitutive model for plane strain and 3D cases
* @author Vahid Galavi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could be re-written a bit more (as you already started doing so): I'd propose something like:
This class defines an incremental linear elastic constitutive model for plane strain and 3D cases.

I don't think that 'small deformation' is really accurate (or has anything to do with the linear elastic model), but please correct me if I'm wrong (tagging you @WPK4FEM, to hear your opinion on this as well)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To stress the incremental nature of the implementation, we could replace "small deformation" with "incremental". The formulation can be used both for small deformation and large deformation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replaced small deformation with incremental

Copy link
Contributor

@WPK4FEM WPK4FEM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to revert my not so to the point questions and agreeing with Richard.

}

bool& GeoLinearElasticPlaneStrain2DLaw::GetValue(const Variable<bool>& rThisVariable, bool& rValue)
bool& GeoIncrementalLinearElasticLaw::GetValue(const Variable<bool>& rThisVariable, bool& rValue)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked at Stenberg stabilization. Only seems applicable for thick shell formulation used in Structural Mechanics Applications. I consider it to be part of another PR to limit its use to meaningful use in GeoMechanics.

Comment on lines 24 to 27
* @class GeoIncrementalLinearElasticLaw
* @ingroup GeoMechanicsApplication
* @brief This class defines a small deformation linear elastic constitutive model for plane strain cases
* @brief This class defines a small deformation linear elastic constitutive model for plane strain and 3D cases
* @author Vahid Galavi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To stress the incremental nature of the implementation, we could replace "small deformation" with "incremental". The formulation can be used both for small deformation and large deformation.

@@ -351,6 +351,7 @@ void KratosGeoMechanicsApplication::Register()
KRATOS_REGISTER_CONSTITUTIVE_LAW("LinearElasticPlaneStrainK02DLaw", mLinearPlaneStrainK0Law)
KRATOS_REGISTER_CONSTITUTIVE_LAW("LinearElasticK03DLaw", mElasticIsotropicK03DLaw)
KRATOS_REGISTER_CONSTITUTIVE_LAW("GeoLinearElasticPlaneStrain2DLaw", mLinearElasticPlaneStrain2DLaw)
KRATOS_REGISTER_CONSTITUTIVE_LAW("GeoLinearElastic3DLaw", mLinearElastic3DLaw)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I share Richards point of view.

Copy link
Contributor

@avdg81 avdg81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very clean and clear extension of the incremental linear elastic law. All of my comments are minor in nature. The one thing I'd like to add is that there are three README.md files that still refer to the previous constitutive law name (i.e. LinearElastic3DLaw):

  • tests/compressibility_tests/README.md
  • tests/hexa_8n_normal_load/README.md
  • tests/lysmer_boundary_column3D/README.md

I guess those need to updated, too.

namespace Kratos
{

class KRATOS_API(GEO_MECHANICS_APPLICATION) Elastic3D : public ConstitutiveLawDimension
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favor of renaming this class to ThreeDimensional. Needless to say that we then also need to rename this header file as well as the corresponding implementation file.

@@ -351,6 +351,7 @@ void KratosGeoMechanicsApplication::Register()
KRATOS_REGISTER_CONSTITUTIVE_LAW("LinearElasticPlaneStrainK02DLaw", mLinearPlaneStrainK0Law)
KRATOS_REGISTER_CONSTITUTIVE_LAW("LinearElasticK03DLaw", mElasticIsotropicK03DLaw)
KRATOS_REGISTER_CONSTITUTIVE_LAW("GeoLinearElasticPlaneStrain2DLaw", mLinearElasticPlaneStrain2DLaw)
KRATOS_REGISTER_CONSTITUTIVE_LAW("GeoLinearElastic3DLaw", mLinearElastic3DLaw)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree to have "Incremental" in the registered name.

@markelov208
Copy link
Contributor Author

This is very clean and clear extension of the incremental linear elastic law. All of my comments are minor in nature. The one thing I'd like to add is that there are three README.md files that still refer to the previous constitutive law name (i.e. LinearElastic3DLaw):

  • tests/compressibility_tests/README.md
  • tests/hexa_8n_normal_load/README.md
  • tests/lysmer_boundary_column3D/README.md

I guess those need to updated, too.

These READMEs are corrected.

Copy link
Contributor Author

@markelov208 markelov208 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi guys, many thanks for your suggestions.

{
// This Constitutive Law has been checked with Stenberg Stabilization
if (rThisVariable == STENBERG_SHEAR_STABILIZATION_SUITABLE) rValue = true;
return rValue;
}

void GeoLinearElasticPlaneStrain2DLaw::GetLawFeatures(Features& rFeatures)
void GeoIncrementalLinearElasticLaw::GetLawFeatures(Features& rFeatures)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed ;) It is used across Kratos.

@@ -53,7 +53,7 @@ class KRATOS_API(GEO_MECHANICS_APPLICATION) LinearElastic2DBeamLaw : public GeoL
using CLBaseType = ConstitutiveLaw;

/// The base class ElasticIsotropicK03DLaw type definition
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

corrected

@@ -53,7 +53,7 @@ class KRATOS_API(GEO_MECHANICS_APPLICATION) LinearElastic2DInterfaceLaw : public
using CLBaseType = ConstitutiveLaw;

/// The base class ElasticIsotropicK03DLaw type definition
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

corrected

KRATOS_EXPECT_VECTOR_RELATIVE_NEAR(expected_stress, stress, 1e-3);
}

KRATOS_TEST_CASE_IN_SUITE(GeoLinearElastic3DLawReturnsExpectedStress_WhenOnlyDiagonalEntriesAreConsidered,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All credits to Richard. I've just applied his tests.

@@ -351,6 +351,7 @@ void KratosGeoMechanicsApplication::Register()
KRATOS_REGISTER_CONSTITUTIVE_LAW("LinearElasticPlaneStrainK02DLaw", mLinearPlaneStrainK0Law)
KRATOS_REGISTER_CONSTITUTIVE_LAW("LinearElasticK03DLaw", mElasticIsotropicK03DLaw)
KRATOS_REGISTER_CONSTITUTIVE_LAW("GeoLinearElasticPlaneStrain2DLaw", mLinearElasticPlaneStrain2DLaw)
KRATOS_REGISTER_CONSTITUTIVE_LAW("GeoLinearElastic3DLaw", mLinearElastic3DLaw)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added Incremental

Copy link
Contributor

@rfaasse rfaasse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for processing the review comments, I only have two very minor nitpicky comments left, but to me this PR is as good as ready!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor typo in the name (inremental -> incremental)

* @ingroup GeoMechanicsApplication
* @brief This class defines a small deformation linear elastic constitutive model for plane strain cases
* @brief This class defines a incremental linear elastic constitutive model for plane strain and 3D cases
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very minor (nitpicking at this point)

Suggested change
* @brief This class defines a incremental linear elastic constitutive model for plane strain and 3D cases
* @brief This class defines an incremental linear elastic constitutive model for plane strain and 3D cases

Copy link
Contributor

@avdg81 avdg81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we're almost there. I just have two very minor suggestions left. Thank you so much for the work done so far. Much appreciated.

Copy link
Contributor

@avdg81 avdg81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge this PR :-)

@markelov208 markelov208 enabled auto-merge (squash) November 8, 2024 09:30
@markelov208 markelov208 merged commit 8cdabbc into master Nov 8, 2024
11 checks passed
@markelov208 markelov208 deleted the geo/12814_3D_elastic_constitutive_law branch November 8, 2024 10:46
@rfaasse
Copy link
Contributor

rfaasse commented Nov 8, 2024

it is none of my bussiness ;) but would it be possible to generalize the existing CLs from the SMApp to satisfy your needs?

Hi @AlejandroCornejo, thanks for reaching out! In principle a 3D linear elastic law is indeed not something that's specific for geomechanics and we could re-use from structural. Right now there are still two reasons why we still have a different version in geo:

  1. The formulation we added is incremental
  2. We need the possibility to only consider diagonal terms in our constitutive matrix and disregard the shear terms (and control from outside of the constitutive law when this happens).

However, since you're of course more familiar with the SMApp, we might have missed possibilities to re-use your CLs, so please let us know if you see possibilities for collaboration here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[GeoMechanicsApplication] A 3D elastic constitutive law is lacking in GeoMechanicsApplication
5 participants